-
Notifications
You must be signed in to change notification settings - Fork 0
Add PROOF ballots to the report #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds “PROOF” ballot PDFs to VxQA runs and exposes them in the generated HTML report to help visually verify bubble-to-contest/option mapping (and catch issues like candidate flip/reorder when totals match).
Changes:
- Generate PROOF ballot PDFs during
runQAWorkflowand include them in the report output directory. - Update HTML report generation to render a “Ballot Proof Gallery” pairing each base ballot PDF with its PROOF counterpart.
- Add PDF snapshot testing utilities (Vitest +
jest-image-snapshot) and snapshot fixtures for proof ballot rendering.
Reviewed changes
Copilot reviewed 9 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Adds a Vitest setup file to register custom matchers. |
| src/test/setup.ts | Extends Vitest expect with toMatchImageSnapshot. |
| src/test/pdf-snapshot.ts | Adds a helper to render PDF pages to images and snapshot-test them. |
| src/report/pdf-thumbnail.ts | Makes PDF thumbnail scale configurable. |
| src/report/html-generator.ts | Builds base/proof ballot pairs and renders a new “Ballot Proof Gallery”. |
| src/cli/config-runner.ts | Generates PROOF-*.pdf alongside each base ballot PDF. |
| src/ballots/proof-ballot.ts | Implements proof ballot annotation overlay generation with pdf-lib. |
| src/ballots/proof-ballot.test.ts | Adds unit + fixture-based snapshot tests for proof ballots. |
| src/ballots/image_snapshots/* | Adds snapshot PNGs for proof ballot fixture rendering. |
| package.json | Adds dev deps for image snapshot testing. |
| pnpm-lock.yaml | Locks new snapshot-testing dependencies. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const FIXTURE_PATH = join( | ||
| import.meta.dirname, | ||
| '../../test-fixtures/election-package-and-ballots-e71c80e-c4446e7.zip', | ||
| ); |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import.meta.dirname is only available in newer Node versions, but this repo's engines.node allows any >=20.0.0. To avoid test failures on Node 20.x versions that don't support it, derive the directory from import.meta.url (via fileURLToPath(new URL('.', import.meta.url))) or tighten the Node engine range to a version that guarantees import.meta.dirname.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this will be set up by vitest.
Overlay contest/candidate labels on each ballot PDF so a human can visually verify that bubble positions are correctly mapped. Proof PDFs are saved alongside base ballots and now appear in the HTML report's new Ballot Gallery section.
Add jest-image-snapshot with a lightweight helper that renders PDF pages via pdf-to-img and compares them pixel-by-pixel, similar to vxsuite's toMatchPdfSnapshot.
Export pure geometry/label functions for direct unit testing. Add PDF snapshot test that renders a synthetic proof ballot and compares against stored baseline images.
Filter gallery to ballots that have a PROOF- counterpart, excluding marked/scanned variants. Display each pair side-by-side for easy comparison. Render gallery thumbnails at 2x scale for readability.
Replace synthetic snapshot test with fixture-based tests that generate proof ballots from the real election package, covering both ballot styles (2-page and 4-page).
e165436 to
7c75f3e
Compare
jonahkagan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
| const ballotStyle2 = electionPackage.ballots.find( | ||
| (b) => b.ballotStyleId === '2_en' && b.ballotMode === 'official' && b.ballotType === 'precinct', | ||
| )!; | ||
| const prooftest = test.extend<{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, didn't know about this feature. I'm thinking about how this might be useful beyond our usual pattern of just directly calling plain helper functions at the start of a test to set up fixtures, but drawing a bit of a blank. Curious if you have any ideas in mind about what this might be useful for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good question. I think the main difference is that it allows scoping overrides in a describe. However, I don't necessarily like that since it adds a fair amount of cognitive overhead. I encountered the same thing ages ago with RSpec and the heavy use of let, particularly in overriding a let in a nested scope. Tracing where the value came from was just too annoying.
This pattern has the same potential for abuse. I do like this over setting global-ish variables in a before* hook, but not necessarily more than just having some helpers called inside the test body. I make use of the pattern quite a bit in the playwright tests in a personal project of mine. I like that the helpers like createAccount and createTransaction don't need to be passed the context like the surreal instance--they can just declare that they need it. This can all of course be handled by more plain helper functions on an object or by a class or whatever instead.
Do I think we should keep this? I don't feel too strongly about it 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It reminds me a bit of the fixture injection system in pytest, which I used a lot in Arlo. I agree it can get annoying managing scopes/overrides. I generally like declarative systems so that part of it appeals to me. I also don't feel strongly about it and am happy to keep it and see how it goes.
We currently generate votes and mark ballots accordingly, then sum up those votes and compare the totals against the totals of all the CVRs. This ensures that the totals are correct for the given marking patterns, but it doesn't protect against candidate bubbles being flipped or reordered if both candidates got the same total.
As a first step in protecting against that case, this change makes VxQA produce PROOF ballots, i.e. copies of the ballots annotated to aid in validating their correctness. They have the candidate/contest option associated with the bubble shown with labels in a green box, the locations we expect the bubbles to be marked with a red X, and the area we inspect for write-in marks in a tan box.
The implementation is based on a previous version that was only ever used on a New Hampshire-specific branch: votingworks/vxsuite#5267. It's modified a bit for style and to suit the slightly different tooling in vx-qa.
Tests pull in
jest-image-snapshotto do PDF snapshotting in a similar manner to how vxsuite does it.Example contest with annotations:
Example PROOF PDF:
PROOF-ballot-2_en-s3y4ua5am2y7-official-absentee.pdf